Something like this gets my vote. WDYT @mik-laj? (It might be worth moving some of that logic down in to the base DbApiHook? Not sure.)

It might be worth moving some of that logic down in to the base DbApiHook? Not sure.

Yeah I thought about this too and was also unsure. Just "connection" is a term that has great potential for conflict / confusion / ambiguity, to add it to all hooks could be trouble. Maybe if we renamed Connection to AirflowConnection... Or perhaps better, Creds or Credential. Connection is not really a connection. Come to think of it, this causes a lot of confusing code (all the get_conn in the codebase).

An alternative approach would be to alter the hook to accept every param the mysql connections (actual connection) would need, so that the GCP Cloud SQL hook does not actually need to create a connection object -- it can just pass the appropriate params to postgres / mysql hook.

On this latter idea, some have discouraged adding params like password as init params for hook on the basis it would allow users to do bad things like put creds in the dag file under source control. I personally don't feel like we should be nanny state in that way. Constrain as we might, a novice user might still put pyodbc.connect in their dags, for example. So I don't see this as a reason not to add params.

OK Just want to offer one more idea.

If we don't like adding connection to mysql hook / postgres hook, we can make simple subclasses in airflow/gcp/hooks/cloud_sql.py. It could even by defined inside CloudSqlDatabaseHook.

So we define something like this:

class _GCPMysqlHook(MySqlHook):
    def __init__(self, connection: Connection, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.connection = connection

    def get_connection(self, conn_id: str) -> Connection:
        return self.connection

And then in get_database_hook it's something like this:

self.db_hook = self._GCPMysqlHook(connection=connection, schema=self.database)

Codecov Report

Merging #6440 into master will decrease coverage by 0.35%.
The diff coverage is 71.42%.

@@            Coverage Diff             @@
##           master    #6440      +/-   ##
==========================================
- Coverage   83.99%   83.63%   -0.36%     
==========================================
  Files         627      627              
  Lines       36537    36516      -21     
==========================================
- Hits        30690    30541     -149     
- Misses       5847     5975     +128

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 171e1bb...a521089. Read the comment docs.

Or perhaps better, Creds or Credential. Connection is not really a connection. Come to think of it, this causes a lot of confusing code (all the get_conn in the codebase).

Oooh interesting idea, Connection usually implies and active connection/open socket, but as you say, our Connection objects are just credentials and connection information. Might be a thing to rename pre 2.0, at least in the code an models, if maybe not in the UI?

@potiuk would you be interested in reviewing?

These GCP hooks create connections in database. Changing this makes it more compatible with this PR allowing users to pull creds from other sources.

Yep. Was on my list . Will do it latest tomorrow.

thanks @potiuk

i have rebased